pinctrl: initial support for GD32 #39974

gmarull · 2021-11-01T22:20:17Z

This PR adds a new pinctrl driver for GD32 SoCs for both AF and AFIO pinmux models.

Tested on:

GD32F450I-EVAL
GD32F403Z-EVAL (@nandojve)

It depends on:

github-actions · 2021-11-01T22:21:25Z

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
hal_gigadevice	zephyrproject-rtos/hal_gigadevice@`85f10a6`	zephyrproject-rtos/hal_gigadevice@`cb4cc10` (main)	zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

gmarull · 2021-11-07T20:34:52Z

@nandojve Support for AFIO (like GD32F403) should now be ready. However, I haven't been able to test it as I don't have that board (will try on VF103 using #34970)

nandojve · 2021-11-08T01:38:12Z

@nandojve Support for AFIO (like GD32F403) should now be ready. However, I haven't been able to test it as I don't have that board (will try on VF103 using #34970)

Great ! I'll try soon and report to you.

nandojve · 2021-11-12T19:31:25Z

Hi @gmarull ,

I tested using gd32f403 without sucess, no characters at serial port.
I copy usart0_default resulting node here to help you check if something is missing.

usart0_default: usart0_default {
	phandle = < 0x2 >;
	pins1 {
		pinmux = < 0x11290 >, < 0x111a0 >;
	};
};

nandojve

Hi @gmarull ,

Excellent work!

I start to look from manifest update and have only small comments.

soc/arm/gigadevice/Kconfig

boards/arm/gd32f450i_eval/gd32f450i_eval_defconfig

drivers/serial/Kconfig.gd32

boards/arm/gd32f403z_eval/gd32f403z_eval_defconfig

nandojve

Hi @gmarull ,

After some fixes AFIO is working:

*** Booting Zephyr OS build v2.7.99-1382-g31b26bab517c  ***
Running test suite pinctrl_gd32
===================================================================
START - test_dt_extract
 PASS - test_dt_extract in 0.1 seconds
===================================================================
Test suite pinctrl_gd32 succeeded
===================================================================
PROJECT EXECUTION SUCCESSFUL

dts/arm/gigadevice/gd32f403/gd32f403.dtsi

drivers/pinctrl/pinctrl_gd32_afio.c

nandojve · 2021-11-13T00:53:38Z

dts/bindings/pinctrl/gd,gd32-pinctrl-common.yaml

bias-high-impedance is missing

This option is not required, same as STM32 (see https://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/pinctrl/st%2Cstm32-pinctrl.yaml#L18). Not setting any bias and operating as input would be sufficient as far as I understand.

Not setting any bias and operating as input would be sufficient as far as I understand.

I understand not setting any bias, which means bias-disable (default state), equals Analog. We should not work with a hidden state because people can made confusion. The configuration table have 4 options for Input and there are 3 options defined. If you look at

zephyr/dts/bindings/pinctrl/pincfg-node.yaml

Lines 25 to 28 in a5c2a61

bias-high-impedance:

required: false

type: boolean

description: high impedance mode ("third-state", "floating")

this is the option that Linux define for the Input Floating.

The problem is that analog is treated via the pinmux value as a separate mode, not as an input mode. In case of DAC (an output signal), ANALOG still has to be used. For low-power mode, what is usually done is to set the analog mode (default config) on a pin, this is why I added the analog option for all pins in the generator:

/* ANALOG */ #define ANALOG_PA0 \ GD32_PINMUX_AFIO('A', 0, ANALOG, NORMP) #define ANALOG_PA1 \ GD32_PINMUX_AFIO('A', 1, ANALOG, NORMP) ...

I know it can be a bit confusing, it's really STM32 inheritance (it's done this way, see for example https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/stm32mp15-pinctrl.dtsi#L62). I think that treating analog in a special way makes things more understandable when it comes to the analog signals, but not that clear for low-power modes. In such case, it would probably make more sense to configure the pin as an input and set bias-high-impedance. But familiarity with STM32 has value for users experienced in that platform.

In this case, it is important give some background for users. The information you presented at last comment is relevant to users understand how this should be configured. Other important thing is that no one can determine from where user will start to read the documentation. I think it could be very valuable improve documentation and try connect those things. For instance, correlate the GPIO configuration table with the options available can be very valuable to users. The same way, explain to then the why Analog is configured differently will help. Maybe provide pointer that exists in Zephyr can help to. Take in consideration that a lot of users don't know Linux and devicetree.

I have updated the bindings description explaining more details on low power modes, please check.

nandojve · 2021-11-13T00:54:07Z

dts/bindings/pinctrl/gd,gd32-pinctrl-af.yaml

bias-high-impedance is missing

bias-disable: Disable pull-up/down (default, not required).

What is this? Analog or High-Impedance? Again, it is not clear the meaning and it is possible improve here to a very clear pin state.

nandojve · 2021-11-13T00:54:44Z

dts/bindings/pinctrl/gd,gd32-pinctrl-afio.yaml

bias-high-impedance is missing

gmarull · 2021-11-14T18:09:41Z

@cameled reviews welcome

nandojve

Hi @gmarull ,

Thank you for this PR!

mbolivar-nordic

I didn't review this very closely, but it looks like it's following all the latest practices from a quick skim.

Include the module `include` folder to allow access to the dt-bindings helpers. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add a pin control driver for GD32 SoCs using the AF model. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add definitions for the pinctrl and gpio nodes. Signed-off-by: Gerard Marull-Paretas <[email protected]>

GD32F4XX series have AF based pinmux. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add a pin control driver for GD32 SoCs using the AFIO model. Thanks to Gerson Fernando Budke for testing and implementation suggestions. Co-authored-by: Gerson Fernando Budke <[email protected]> Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add afio, pinctrl and gpioa...g entries for the GD32F403 series. Signed-off-by: Gerard Marull-Paretas <[email protected]>

GD32F403 series use AFIO pinmux model. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Enable pinctrl by default, since it is an essential component on almost every firmware. Inclusion of series defconfig has also been guarded with SoC availability (was missing). Signed-off-by: Gerard Marull-Paretas <[email protected]>

Use the pinctrl API to configure peripheral pins. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Enable usage of the pinctrl driver. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Enable usage of pinctrl. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add a test to check that DT information for AF model is extracted correctly. Signed-off-by: Gerard Marull-Paretas <[email protected]>

Add a test to check that DT information for the AFIO model is extracted correctly. Signed-off-by: Gerard Marull-Paretas <[email protected]>

gmarull requested a review from nandojve November 1, 2021 22:20

github-actions bot added area: Boards area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Modules manifest west manifest-hal_gigadevice labels Nov 1, 2021

gmarull force-pushed the gd32-pinctrl branch from ed9a4e9 to 23ec648 Compare November 1, 2021 22:26

gmarull mentioned this pull request Nov 2, 2021

[RFC] GigaDevice Vendor Support #38657

Open

55 tasks

gmarull force-pushed the gd32-pinctrl branch from 23ec648 to fd50776 Compare November 4, 2021 22:27

nandojve added the platform: GigaDevice label Nov 6, 2021

gmarull force-pushed the gd32-pinctrl branch 2 times, most recently from 43d42a5 to f87f1b7 Compare November 7, 2021 20:32

gmarull force-pushed the gd32-pinctrl branch from f87f1b7 to 58235a1 Compare November 7, 2021 22:50

nandojve requested changes Nov 12, 2021

View reviewed changes

nandojve requested changes Nov 13, 2021

View reviewed changes

gmarull force-pushed the gd32-pinctrl branch 2 times, most recently from e4690d7 to 4799cd9 Compare November 13, 2021 17:30

gmarull requested a review from nandojve November 13, 2021 17:30

gmarull force-pushed the gd32-pinctrl branch 2 times, most recently from 3b01287 to c8f784d Compare November 14, 2021 18:06

github-actions bot added the DNM This PR should not be merged (Do Not Merge) label Nov 14, 2021

gmarull force-pushed the gd32-pinctrl branch from c8f784d to d49b8eb Compare November 14, 2021 18:12

gmarull requested review from MaureenHelm, carlescufi, galak and mbolivar-nordic November 18, 2021 19:42

gmarull force-pushed the gd32-pinctrl branch from f9374d2 to 9ed980d Compare November 18, 2021 19:43

github-actions bot removed the DNM This PR should not be merged (Do Not Merge) label Nov 18, 2021

nandojve approved these changes Nov 18, 2021

View reviewed changes

mbolivar-nordic approved these changes Nov 20, 2021

View reviewed changes

gmarull force-pushed the gd32-pinctrl branch from 9ed980d to 1f9d963 Compare November 22, 2021 10:33

gmarull and others added 13 commits November 22, 2021 16:46

modules: hal_gigadevice: include module headers

f9e0c7a

Include the module `include` folder to allow access to the dt-bindings helpers. Signed-off-by: Gerard Marull-Paretas <[email protected]>

drivers: pinctrl: gd32: initial support for AF based SoCs

a1faabe

Add a pin control driver for GD32 SoCs using the AF model. Signed-off-by: Gerard Marull-Paretas <[email protected]>

dts: arm: gd32f4xx: add pinctrl and gpio nodes

cbefb52

Add definitions for the pinctrl and gpio nodes. Signed-off-by: Gerard Marull-Paretas <[email protected]>

soc: arm: gigadevice: gd32f4xx: select AF pinmux

e42de1e

GD32F4XX series have AF based pinmux. Signed-off-by: Gerard Marull-Paretas <[email protected]>

dts: arm: gigadevice: gd32f403: add afio and pinctrl

307b600

Add afio, pinctrl and gpioa...g entries for the GD32F403 series. Signed-off-by: Gerard Marull-Paretas <[email protected]>

soc: arm: gigadevice: gd32f403: select AFIO pinmux

aea9d02

GD32F403 series use AFIO pinmux model. Signed-off-by: Gerard Marull-Paretas <[email protected]>

soc: arm: gigadevice: enable pinctrl by default

92054b6

Enable pinctrl by default, since it is an essential component on almost every firmware. Inclusion of series defconfig has also been guarded with SoC availability (was missing). Signed-off-by: Gerard Marull-Paretas <[email protected]>

drivers: serial: gd32: use pinctrl

aee0268

Use the pinctrl API to configure peripheral pins. Signed-off-by: Gerard Marull-Paretas <[email protected]>

boards: gd32f450i_eval: use pinctrl

4dfd336

Enable usage of the pinctrl driver. Signed-off-by: Gerard Marull-Paretas <[email protected]>

boards: gd32f403z_eval: use pinctrl

436447d

Enable usage of pinctrl. Signed-off-by: Gerard Marull-Paretas <[email protected]>

tests: drivers: pinctrl: gd32: add DT AF parse test

b74a907

Add a test to check that DT information for AF model is extracted correctly. Signed-off-by: Gerard Marull-Paretas <[email protected]>

tests: drivers: pinctrl: gd32: add DT AFIO parse test

148b495

Add a test to check that DT information for the AFIO model is extracted correctly. Signed-off-by: Gerard Marull-Paretas <[email protected]>

gmarull force-pushed the gd32-pinctrl branch from 1f9d963 to 148b495 Compare November 22, 2021 15:47

nashif merged commit 1123b80 into zephyrproject-rtos:main Nov 22, 2021

This was referenced Nov 29, 2021

boards: Add support for the raspberrypi pico #34835

Merged

Road from pinmux to pinctrl #39740

Closed

gmarull deleted the gd32-pinctrl branch November 29, 2021 19:42

danieldegrasse mentioned this pull request Feb 3, 2022

Alternative pin group based pinctrl implementation for RT1062 #42238

Merged

danieldegrasse mentioned this pull request Feb 23, 2022

Initial Kinetis pinctrl Implementation #43033

Merged

str4t0m added the platform: GD32 GigaDevice label Aug 26, 2022

	bias-high-impedance:
	required: false
	type: boolean
	description: high impedance mode ("third-state", "floating")

pinctrl: initial support for GD32 #39974

pinctrl: initial support for GD32 #39974

Uh oh!

Conversation

gmarull commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmarull commented Nov 7, 2021

Uh oh!

nandojve commented Nov 8, 2021

Uh oh!

nandojve commented Nov 12, 2021

Uh oh!

nandojve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nandojve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmarull Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmarull commented Nov 14, 2021

Uh oh!

nandojve left a comment

Choose a reason for hiding this comment

Uh oh!

mbolivar-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gmarull commented Nov 1, 2021 •

edited

Loading

github-actions bot commented Nov 1, 2021 •

edited

Loading

gmarull Nov 15, 2021 •

edited

Loading